Compiled property path function #178

DanielTrommel · 2025-07-11T19:56:29Z

@w-ahmad While working on the other small PR (the one to support string-based indexers), I figured the code to build that PropertyInfo list from the Binding's PropertyPath can be implemented completely different, to make it easier and faster (for evaluation):

Changed the code to build a lambda expression tree,
Which is compiled and stored
The resulting function is then used in the GetCellContent to do the evaluation of the binding

Pro's of this approach:

Should be faster during the evaluation (no more looping + pi.GetValue() calls involved; just a compiled function)
Less code! Previous code worked with two GetValue(..) methods that needed to be kept in sync. Now there is only one place where the logic resides to interpret the PropertyPath

Only downside is that (most likely) the initial building + compiling might be more costly, although I did not experience any difference on my implementation project(s). Typically, the PropertyPaths are rather small.

Tested this with my own tables and data, and tested it against your TableView sample app (where I filed a seemingly unrelated bug)

w-ahmad · 2025-07-16T11:54:56Z

Thank you so much for these changes, @DanielTrommel. I tried some benchmarking on these changes, and they are indeed fast. However, I'm a bit concerned because these chagnes are effecting core functionality therfore it would be good to release these in a preview release first. So, let's hold off on these changes for the v1.4 preview release. Thanks again!

DanielTrommel · 2025-07-16T12:18:52Z

@w-ahmad sounds like a safe plan. Happy to fix any bug reports once it gets merged; just tag me.
And thanks for validating the speedup with a benchmark!

…e reflection approach Signed-off-by: Daniël Trommel <[email protected]>

DanielTrommel · 2025-07-16T19:23:05Z

PS: noticed that my commits were commited under my (work) email account, which is not handy. I recommited the changes (squashed the two commits into one), and force-pushed the changes. Nothing else changed.

Copilot

Pull Request Overview

This PR refactors the property path evaluation mechanism to use compiled lambda expressions instead of reflection-based property access. The change replaces the existing reflection-based GetValue methods with a compiled function approach that builds expression trees and compiles them for faster runtime evaluation.

Key changes:

Replaced reflection-based property path evaluation with compiled lambda expressions
Eliminated the need to maintain synchronized GetValue methods
Simplified the caching mechanism in TableViewBoundColumn

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File	Description
src/Extensions/ObjectExtensions.cs	Replaced reflection-based `GetValue` methods with `GetFuncCompiledPropertyPath` that builds and compiles expression trees
src/Columns/TableViewBoundColumn.cs	Updated to use the new compiled property path function, removing type-based caching logic

src/Extensions/ObjectExtensions.cs

Signed-off-by: Daniël Trommel <[email protected]>

DanielTrommel · 2025-08-02T13:35:52Z

@w-ahmad just processed the comments!

w-ahmad · 2025-08-04T10:54:27Z

@w-ahmad just processed the comments!

Thanks @DanielTrommel!

…perty-path-function # Conflicts: # src/Extensions/ObjectExtensions.cs

DanielTrommel · 2025-08-05T06:29:31Z

@w-ahmad when are you planning to merge the UnitTest branch? Happy to integrate those unit tests, and update them to accomodate this new approach, so you can see it works for those tests.

…ew compiled function approach. Note: the support for Dictionary Signed-off-by: Daniël Trommel <[email protected]>

w-ahmad · 2025-08-05T10:41:48Z

@w-ahmad when are you planning to merge the UnitTest branch? Happy to integrate those unit tests, and update them to accomodate this new approach, so you can see it works for those tests.

It's merged!

DanielTrommel · 2025-08-05T12:32:27Z

@w-ahmad just pushed the changes; few notes:

Since the function generating method is all there basically is, I merged each pair of test methods that you introduced into just the one test
I noticed that you added support for IDictionary in the `current' code (on main branch), as part of your UnitTest PR. Without me adding explicit support for that within this PR, the tests still run fine. This has to do with two things: (1) the expression tree approach is much simpler because a Dictionary lookup is in essence still accessing a property with an indexer step after it (see the Dictionary implementation; it is just an indexer) , and (2) I do not do in-code evaluation (only via the compiled function in intermediate steps) so I do not need explicit instances of dictionary or list, like as is needed in the explicit code of the current main branch.

…erty-path-function

w-ahmad

hey @DanielTrommel, thanks for the updates. I ran two more tests, and they failed. These scenarios involve a compiled function that fails when invoked with a different data item. Please try these yourself. This fix is necessary since, in real-world usage, there's no guarantee we'll always get the same data items.

[TestMethod]
public void GetFuncCompiledPropertyPath_ShouldReturnNull_ForSimpleNestedProperty()
{
    var testItem = new TestItem { SubItems = [new() { SubSubItems = [new() { Name = "NestedValue" }] }] };
    var func = testItem.GetFuncCompiledPropertyPath("SubItems[0].SubSubItems[0].Name");
    Assert.IsNotNull(func);

    var result = func(testItem);
    Assert.AreEqual("NestedValue", result);

    testItem = new TestItem { SubItems = [new() { SubSubItems = null! }] };
    result = func(testItem);
    Assert.IsNull(result);
}

[TestMethod]
public void GetFuncCompiledPropertyPath_ShouldReturnNull_ForInvalidIndexer2()
{
    var testItem = new TestItem { Dictionary2 = new() { { 1, "value1" } } };
    var func = testItem.GetFuncCompiledPropertyPath("Dictionary2[1]");
    Assert.IsNotNull(func);

    var result = func(testItem);
    Assert.AreEqual("value1", result);

    testItem = new TestItem { Dictionary2 = new() { { 2, "value2" } } };
    result = func(testItem);
    Assert.IsNull(result);
}

Signed-off-by: Daniël Trommel <[email protected]>

…ression/Lambda code with DebugView Signed-off-by: Daniël Trommel <[email protected]> (cherry picked from commit 5c0ccd2b9be1866427b74c91d752f8fefa4b88b4)

…soon as one step yields null, to guard against non-complete property paths for some instances Signed-off-by: Daniël Trommel <[email protected]>

…tests for all possible cases Signed-off-by: Daniël Trommel <[email protected]>

DanielTrommel · 2025-08-07T19:52:30Z

@w-ahmad thanks for those tests. Based off this, I did a refactor, to add support for these, and various other things:

Generic support for multi-dimensional indexers, typically 2D arrays I suppose. Also added tests for those
General null checking at each `step', to return null if we hit an too-early null (and thus should not continue the evaluation)
Refined the support For IDict/Ilist/ICollection, and a fallback for indexers where we eventually use a try-catch to make sure we intercept failing indexer lookups

Added various test cases to support the code (in the next commits)

Signed-off-by: Daniël Trommel <[email protected]>

…nnot be bound-checked, and do not have a TryGetValue construct like with dictionary. We could add code to search for such a method, but there is not guarentee that is the method we should use... Signed-off-by: Daniël Trommel <[email protected]>

- An explicit conversion is not always needed when an implicit conversion would take care of it - The in-loop compilation (used to determine the need for conversion) is not needed for the last match (Typically, Binding path is just accessing one property, and thus the inloop compilation will not be used, saving some time) Signed-off-by: Daniël Trommel <[email protected]>

…nement of the conversion logic

DanielTrommel · 2025-08-09T13:33:14Z

@w-ahmad ok, it feels quite complete now. Quite some code added, but also more supported scenario's.

One thing that is still in the back of my mind, is that both the current pis as well as this Func<> approach are vulnerable to that 'first' dataItem with which the current pis array (or the Func<> here) are being built. Could very well be that that first instance is not representative (i.e. at some point the binding path fails to execute), and the result is null.
I suppose in either case, the code will automatically retry building the pis/Func<> for the next items (because it stays null), until something rolls out that is not null.

It feels like it would be better to do the derivation Type-based, and take into account that the rows could be a mixture of instances of different types..? (would be good to test how 'flexible' the binding path in WinUI is to see what could be provided as data...)

And also, one would need to take into consideration that due to subclassing, the binding path might yield different type-paths in effect, with (depending on the instance) might need different PropertyInfo's along the way, for the same part in the binding path...
(The PI's that you determine for the pis array, are limited to the type (subclass) they are determined on, and will fail to evaluate when a different subclass is the result of a part of the binding path.)

w-ahmad · 2025-08-18T22:29:36Z

Thank you so much for the thoughtful improvements and for covering more scenarios. I really appreciate the effort put in and adding more unit tests to ensure everything works smoothly across different cases.

Apologies for my late response. I was testing a different approach where we could use the binding itself to elevate the property values, instead of relying on reflection or compiled lambda expressions. The idea behind this is that our current implementation does not cover all Binding features. For example, we are not considering the fallback properties, or cases where Source or ElementName is specified in the Binding, or situations where users have their own Binding implementations.

By leveraging the binding mechanism, we could potentially resolve all these issues. I know this approach is not as performant as reflection or compiled lambda expressions, but after testing with 100k items, I managed to find a method that takes less time than my initial tests from the early days of TableView. It takes less than a second, and that is acceptable if it allows us to handle all edge cases reliably.

DanielTrommel · 2025-08-19T05:28:36Z

@w-ahmad no worries, always good to choose the better approach!

A while back, I did look into whether the Binding resolving mechanism was exposed, but thought it was not.
Did you find a way? I am curious!

DanielTrommel mentioned this pull request Jul 14, 2025

Is WinUI.TableView suitable for AOT ? #140

Open

Compiled lambda expression, which is faster than the previous GetValu…

d932f8c

…e reflection approach Signed-off-by: Daniël Trommel <[email protected]>

DanielTrommel force-pushed the compiled-property-path-function branch from fc10df0 to d932f8c Compare July 16, 2025 19:19

w-ahmad requested a review from Copilot August 2, 2025 12:19

Copilot AI reviewed Aug 2, 2025

View reviewed changes

Processed Copilot review comments on Github

323e9aa

Signed-off-by: Daniël Trommel <[email protected]>

Merge remote-tracking branch 'remotes/origin/tests' into compiled-pro…

da5a8d5

…perty-path-function # Conflicts: # src/Extensions/ObjectExtensions.cs

Updated unit test previous GetValue() reflection approach, with the n…

067465b

…ew compiled function approach. Note: the support for Dictionary Signed-off-by: Daniël Trommel <[email protected]>

Merge remote-tracking branch 'remotes/origin/main' into compiled-prop…

e1ba188

…erty-path-function

w-ahmad requested changes Aug 6, 2025

View reviewed changes

DanielTrommel added 6 commits August 7, 2025 11:14

Simplification of the indexer code

34cc106

Signed-off-by: Daniël Trommel <[email protected]>

Generic support for multi-dimensional indexers

99d2f62

Signed-off-by: Daniël Trommel <[email protected]>

Added NULL checks for Arrays

dc07aa7

Signed-off-by: Daniël Trommel <[email protected]>

More performant approach that is more readable when reviewing the Exp…

0f6a967

…ression/Lambda code with DebugView Signed-off-by: Daniël Trommel <[email protected]> (cherry picked from commit 5c0ccd2b9be1866427b74c91d752f8fefa4b88b4)

Added Null-check code for every property access step; return null as …

5343d41

…soon as one step yields null, to guard against non-complete property paths for some instances Signed-off-by: Daniël Trommel <[email protected]>

Added logic for IDict/IList/ICollection/generic indexer, but not yet …

c623f92

…tests for all possible cases Signed-off-by: Daniël Trommel <[email protected]>

DanielTrommel added 5 commits August 8, 2025 07:38

Added the simplest test; access a property directly on the instance

fd1bf26

Signed-off-by: Daniël Trommel <[email protected]>

Test case for List, and edge cases where null should be returned

0c2b84c

Signed-off-by: Daniël Trommel <[email protected]>

Another test case; accessing a property on a string. And further refi…

dfa60d6

…nement of the conversion logic

Uh oh!

Compiled property path function #178

Are you sure you want to change the base?

Compiled property path function #178

Uh oh!

Conversation

DanielTrommel commented Jul 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

w-ahmad commented Jul 16, 2025

Uh oh!

DanielTrommel commented Jul 16, 2025

Uh oh!

DanielTrommel commented Jul 16, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

DanielTrommel commented Aug 2, 2025

Uh oh!

w-ahmad commented Aug 4, 2025

Uh oh!

DanielTrommel commented Aug 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

w-ahmad commented Aug 5, 2025

Uh oh!

DanielTrommel commented Aug 5, 2025

Uh oh!

w-ahmad left a comment

Choose a reason for hiding this comment

Uh oh!

DanielTrommel commented Aug 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DanielTrommel commented Aug 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

w-ahmad commented Aug 18, 2025

Uh oh!

DanielTrommel commented Aug 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

DanielTrommel commented Jul 11, 2025 •

edited

Loading

DanielTrommel commented Aug 5, 2025 •

edited

Loading

DanielTrommel commented Aug 7, 2025 •

edited

Loading

DanielTrommel commented Aug 9, 2025 •

edited

Loading

DanielTrommel commented Aug 19, 2025 •

edited

Loading